PB-946 part 2: backfill test for stack notification batcher#776
Conversation
| } | ||
|
|
||
| // Wait for ticker to flush (interval is 100ms) | ||
| time.Sleep(250 * time.Millisecond) |
There was a problem hiding this comment.
Do we have a better mechanism for ensuring the notifications have flushed?
Could we take advantage of https://pkg.go.dev/testing/synctest ?
There was a problem hiding this comment.
TIL synctest, I gave it a good play. I don't think it works in our use. The basic premise of synctest is like poorman's DST, within synctest.Test's when all goroutines are "durably blocked", time flies fast, it fast track to a time where at least one goroutine can be blocked, and then fast forward to next etc. As if a single threaded cooperative concurrency model.
The test goroutine can also enter durably blocked state by issuing time.Sleep or synctest.Wait.
Sadly "durably blocked" does not include:
System calls like file or network I/O
External event handling (such as reading from a socket)
This is precisely what the agent faker server is doing.
As a result, our fake agent server (blocked on the real IO connection accept) and client (blocked on readLoop/writeLoop) will never enter the "durably blocked" state, meaning the entire synctest block will be blocked. (Remember, it's single threaded, one running routine can cause the entire thread to block).
I think it's in theory possible to instrument fake agent server and client so it can become "durably blocked" during test, but I don't think the benefits justify the effort yet.
2193383 to
6e5486a
Compare
6e5486a to
7a787ca
Compare
Back fill test coverage for notification batcher.